chore(lint): enforce tailwind design tokens#241
Conversation
Amp-Thread-ID: https://ampcode.com/threads/T-019d3dfc-73e0-77e8-b0f2-7a46ec585e4f Co-authored-by: Amp <amp@ampcode.com>
zainfathoni
left a comment
zainfathoni
left a comment
There was a problem hiding this comment.
PR Review: chore(lint): enforce tailwind design tokens
✅ ESLint Plugin Configuration — Looks Good
tailwindcsscorrectly added topluginsarraytailwindcss/no-arbitrary-value: "error"properly enforces design token usagesettings.tailwindcss.configcorrectly points totailwind.config.js- Choosing a single rule (
no-arbitrary-value) instead ofextends: ["plugin:tailwindcss/recommended"]is a reasonable scoped approach
✅ Pre-commit Enforcement — Already Covered
No changes needed here — the existing lint-staged config already runs eslint --cache --fix on *.{js,jsx,ts,tsx,yml,yaml}, so the new rule is automatically enforced on pre-commit via Husky. Good that the PR didnt add unnecessary config.
⚠️ Minor Observations
-
Version range vs pinning: The PR description says the plugin is "pinned to a version compatible," but
package.jsonuses^3.5.2(caret range), which allows3.x.xupgrades. The lockfile pins it to3.5.2today, but a futurenpm installcould pull a newer minor. If strict pinning is intended, use"3.5.2"(no caret). Low risk since lockfile is committed — just a consistency note. -
Existing code compliance: The
no-arbitrary-valuerule is strict — it rejects all arbitrary Tailwind classes likebg-[#fff],w-[200px],text-[14px], etc. The diff shows no source file changes, which means either:- The codebase already uses no arbitrary values (great!) ✅
- Or existing violations will cause
npm run lintto fail immediately ❌
The PR description mentions verification via
npm run lintpassing, so this seems fine — but worth double-checking CI passes before merge. -
Lock file churn: The diff includes some
postcss-jsandyamlpackage hoisting changes (moved fromtailwindcss/node_modules/to top-level). This is normal npm deduplication behavior — not a concern, just noting it.
Verdict
Clean, minimal, well-scoped change. The configuration is correct and pre-commit enforcement works through existing infrastructure. No breaking changes if the codebase already passes the new rule (which the PR verification suggests it does). 👍
wheeljackz
left a comment
wheeljackz
left a comment
There was a problem hiding this comment.
I validated the Tailwind rule against the current repo patterns and isolated ESLint repros. I did not find any existing arbitrary-value classes in app/, but I did find two issues with the proposed enforcement: it misses our local classNames() helper, and it flags valid Tailwind arbitrary variants like data-[state=open]:pb-8. Posting those inline below.
| "import/order": "error", | ||
| "react/react-in-jsx-scope": ["off"] | ||
| "react/react-in-jsx-scope": ["off"], | ||
| "tailwindcss/no-arbitrary-value": "error" |
There was a problem hiding this comment.
tailwindcss/no-arbitrary-value is broader than "design token enforcement" here: it also rejects valid Tailwind arbitrary variants/modifiers, not just raw values. I reproduced this locally with <div className="data-[state=open]:pb-8" />, which fails with Arbitrary value detected in data-[state=open]:pb-8. Because this repo already depends on Headless UI, that is a realistic false-positive path for standard Tailwind state selectors, and it will force inline disables as soon as we adopt them.
| "react": { | ||
| "version": "detect" | ||
| }, | ||
| "tailwindcss": { |
There was a problem hiding this comment.
This doesn’t fully enforce the rule across the current codebase because the plugin’s default callees list does not include our local classNames() helper. I counted 32 classNames( call sites in app/, and I reproduced that className={classNames(bg-[#fff], p-4)} passes lint while the same sample with clsx(...) fails. Please add settings.tailwindcss.callees with classNames (or migrate those call sites), otherwise arbitrary values still slip through many existing composition paths.
wheeljackz
left a comment
wheeljackz
left a comment
There was a problem hiding this comment.
Found 1 blocking gap in the new Tailwind lint setup. correctly catches direct JSX class strings and calls, but the current config does not cover this repo's helper, so a large share of composed classes can still bypass the rule. I verified that passes lint while the same class in JSX or fails. I did not find existing bracketed Tailwind classes in the repo, so I don't see an immediate false-positive issue on current code, but enforcement is incomplete until the helper is included.
| "react": { | ||
| "version": "detect" | ||
| }, | ||
| "tailwindcss": { |
There was a problem hiding this comment.
The plugin only inspects call expressions whose callee is listed in settings.tailwindcss.callees, and its default list is just classnames, clsx, and ctl. This repo builds many class strings with classNames(...) (for example app/components/button-link.tsx and app/components/page-link.tsx), so arbitrary values inside those calls currently bypass the new rule. I verified that className={classNames("bg-[#fff]", "p-4")} passes lint while the same class in JSX or clsx(...) fails. Please add classNames here or the enforcement will miss a substantial part of the codebase.
2 participants
Summary
eslint-plugin-tailwindcsspinned to a version compatible with the repo's current ESLint and Tailwind setuplint-stagedso arbitrary classes likebg-[#fff]are rejectedVerification
npm run lintnpm testprintf 'export function Demo(){return <div className="bg-[#fff] p-4" />}\n' | npx eslint --stdin --stdin-filename app/__tailwind-smoke.tsxnpx lint-stagedwith a temporary staged TSX file containingbg-[#fff], confirmed failure fromtailwindcss/no-arbitrary-value